Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add blockSeparator option to clipboardTextSerializer core extension #5019

Merged
merged 3 commits into from
Apr 8, 2024
Merged

Add blockSeparator option to clipboardTextSerializer core extension #5019

merged 3 commits into from
Apr 8, 2024

Conversation

hivokas
Copy link
Contributor

@hivokas hivokas commented Mar 29, 2024 •
edited
Loading

Please describe your changes

This PR adds blockSeparator option to clipboardTextSerializer core extension.

It will allow to override block separator (without copy pasting the entire extension from core) and add it to my editor.

import { extensions } from '@tiptap/core';

const editor = new Editor({
  extensions: [
    extensions.ClipboardTextSerializer.configure({
      blockSeparator: '\n',
    }),
  ],
});

How did you accomplish your changes

By making it possible to provide blockSeparator option to clipboardTextSerializer core extension.

How have you tested your changes

I've copied the implementation from this PR to my project and tested it with and without blockSeparator parameter.

How can we verify your changes

By doing this:

import { extensions } from '@tiptap/core';

const editor = new Editor({
  extensions: [
    extensions.ClipboardTextSerializer.configure({
      blockSeparator: '\n',
    }),
  ],
});

Remarks

If there is a better way to implement it, I'm open to it.

Checklist

  • The changes are not breaking the editor
  • Added tests where possible
  • Followed the guidelines
  • Fixed linting issues

Related issues

N/A

@netlify Netlify
Copy link

netlify bot commented Mar 29, 2024 •
edited
Loading

✅ Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit 9c40720
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/6613c340f68ec40008b28e22
😎 Deploy Preview https://deploy-preview-5019--tiptap-embed.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@hivokas hivokas marked this pull request as ready for review March 29, 2024 11:12
@bdbch
Copy link
Member

bdbch commented Apr 5, 2024

Nice idea!

const editor = new Editor({
  extensions: [
    extensions.ClipboardTextSerializer.configure({
      blockSeparator: '\n',
    }),
  ],
});

The extensions.ClipboardTextSerializer.configure approach looks weird though - since this is a core extension, maybe we should add the option to the editors EditorOptions object?

tiptap/packages/core/src/Editor.ts

Lines 56 to 78 in 6f6e49e

public options: EditorOptions = {
element: document.createElement('div'),
content: '',
injectCSS: true,
injectNonce: undefined,
extensions: [],
autofocus: false,
editable: true,
editorProps: {},
parseOptions: {},
enableInputRules: true,
enablePasteRules: true,
enableCoreExtensions: true,
onBeforeCreate: () => null,
onCreate: () => null,
onUpdate: () => null,
onSelectionUpdate: () => null,
onTransaction: () => null,
onFocus: () => null,
onBlur: () => null,
onDestroy: () => null,
}

That way users would not need to do some weird extension-merging-shenanigans to get the option down to the core extensions.

clipboardBlockSeparator: '\n' would be the option to set the option for the extension then?

@hivokas
Copy link
Contributor Author

hivokas commented Apr 8, 2024

clipboardBlockSeparator: '\n' would be the option to set the option for the extension then?

Yes, this approach looks good to me. Will you implement this change?

bdbch
bdbch reviewed Apr 8, 2024
View reviewed changes
packages/core/src/Editor.ts
@@ -235,7 +238,17 @@ export class Editor extends EventEmitter<EditorEvents> {
* Creates an extension manager.
*/
private createExtensionManager(): void {
const coreExtensions = this.options.enableCoreExtensions ? Object.values(extensions) : []

const coreExtensions = this.options.enableCoreExtensions ? [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we only have less than 10 core extensions I'd think it's okay to just load them manually here – that way we can pass configurations down to them.

@bdbch
Copy link
Member

bdbch commented Apr 8, 2024

@hivokas I added a commit that adds coreExtension options to the editor options props.

@bdbch
Copy link
Member

bdbch commented Apr 8, 2024

@svenadlung since I also made changes here it would be best if you could also just review again :)

@bdbch bdbch merged commit b710783 into ueberdosis:main Apr 8, 2024
14 checks passed
@hivokas
Copy link
Contributor Author

hivokas commented Apr 8, 2024

Thanks @bdbch!

This was referenced May 26, 2024
Open
Open
@patooworld patooworld mentioned this pull request May 29, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bdbch bdbch bdbch left review comments

@svenadlung svenadlung svenadlung approved these changes

Assignees
No one assigned
Labels
None yet
Projects
No open projects
Tiptap
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hivokas @bdbch @svenadlung